Skip to content

Bounds Widening: Fix handling of bounds variables for nt_array_ptrs #898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 18, 2020

Conversation

mgrang
Copy link

@mgrang mgrang commented Aug 12, 2020

There were 3 issues with the handling of variables used in the declared bounds
of an nt_array_ptr:

  1. Bounds variables were being determined only for nt_array_ptrs declared in
    the current block. As a result, if a variable is killed in a block then the
    widened bounds in the successor blocks could not be killed. This is fixed by
    storing the bounds variables along with the nt_array_ptrs for the entire
    function instead of per block.

  2. Bounds variables for function parameters were not getting determined
    correctly. This is fixed by uniforming the logic to determine bounds
    variables for function parameters and local variables.

  3. Bounds declared as CountBoundsExpr were not being handled. This is fixed by
    reading the NormalizedBounds instead of invoking ExpandBoundsToRange.

Also performed minor NFC code cleanups.

Fixes #895

…ptrs

There were 2 issues with determination of variables used in the declared bounds
of an _Nt_array_ptr:

1. Bounds variables were being determined only for _Nt_array_ptrs declared in
the current block. As a result, if another block kills a particular variable
then the widened bounds in the successor blocks could not be killed.

2. Bounds variables for function parameters were not getting determined
correctly.

Also performed minor NFC code cleanups.

Fixes #895
Mandeep Singh Grang added 2 commits August 11, 2020 18:50
@mgrang mgrang changed the title Bounds Widening: Fix determination of bounds vars for _Nt_array_ptrs Bounds Widening: Fix handling of bounds variables for nt_array_ptrs Aug 12, 2020
Copy link
Member

@dtarditi dtarditi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good. I had some questions about the clean up. It was a little difficult to follow the clean up intermixed with the bug fix. It is helpful to keep them separate.

@@ -490,7 +489,7 @@ void f20() {
// CHECK: [B9]
// CHECK: upper_bound(q) = 1

// Declared bounds (INT_MIN) and deref offset (INT_MAX - 1). No sequential deref tests. No widening.
// Declared bounds
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why you deleted the rest of the comment. Same goes for other changes in this file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this. The comments should not have been deleted. Not sure how that happened. I will fix this.

Mandeep Singh Grang added 2 commits August 16, 2020 13:29
Early calculation of bounds vars for nt_array_ptrs with invalid bounds caused
two checkedc runtime tests to fail. Moving the calculation of bounds vars to
FillGenSetForEdge fixes this as this function is not invoked for nt_array_ptrs
with invalid bounds.

Also fixed indentation and comments in tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assignments within conditional expressions should kill widened bounds
3 participants